Test/add tests for groups notes services - #30
Conversation
|
💩 Code linting failed, use |
Coverage Report
Summary
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
452ae37 to
ed4deb3
Compare
|
💩 Code linting failed, use |
1 similar comment
|
💩 Code linting failed, use |
9aef092 to
91c14da
Compare
petrCher
left a comment
petrCher
left a comment
There was a problem hiding this comment.
пока ревью не закончен, посмотрел только базовый функционал, когда полностью закончу ревью напишу в тг
пока что буду периодически кидать новые комменты
|
ответ на твой вопрос из коммента к пр: изначально да, планировалось, что одной ручкой будем создавать типы модалок, но сейчас вероятно по причине ненадобности надо будет это убирать единственное, не совсем понял зачем этот вопрос, просто из интереса или ты конкретно спрашивал для реализации чего-то? |
Просто я когда писал тесты думал, делать один тест на все 5 ручек, потому что они однотипные или 5 тестов на каждую отдельно. Заглянул в ТЗ, чтобы принять решение и соответственно сделал один тест. Но на всякий случай это подсветил, потому что получается несовпадение. |
There was a problem hiding this comment.
постарайся распространить мои комменты по кастомному методу и тд на все тесты, не стал прям везде писать одно и тоже
посмотрел пока поверхностно test_notes и conftest, позже подробнее изучу
и еще коммент для себя, чтобы не забыть: не нравится удаление объекта в самом тесте (подумать как упаковать в фикстуру)
petrCher
left a comment
petrCher
left a comment
There was a problem hiding this comment.
так, по сути все отревьюил, но решил еще клоду дать поревьюить (файл с ревью кину в тг), можешь посмотреть что он думает по этому поводу, всему верить у него точно не надо, надо перепроверять, но мало ли, он иногда действительно хорошие решения предлагает
|
и еще на будущее рекомендация, когда делаешь очень много коммитов и пушишь разом больше 5, лучше из них создавать один коммит для пуша, а то много коммитов неудобно, так как ветка засоряется |
|
еще было бы неплохо структурировать тесты по модели crud |
|
💩 Code linting failed, use |
|
изменения вижу, попозже проверю) |
|
если честно, то уже запутался в коде из-за слишком общего пр, готов просто так вмерджить сейчас, только сперва удали note_type тесты и фикстуры к ним на будущее: лучше делать больше пр'ов но для каждой конкретной фичи, то есть для отдельного route на тесты свой пр, иначе это становится абсолютно нечитаемым. Плюс главное не количество кода, а качество, меньше кода всегда лучше, без необходимости сразу не надо писать очень много, всегда можно что-то оставить на потом для доработки |
|
нужно сделать rebase и подтянуть все изменения с моего последнего коммита в main, потом обязательно проверь все ли добавилось |
…бежать пападания отрицательных значений в limit и offset, и не пропускать не валидные строки в status
- Исправлены ошибки в сигнатуре ручки get_notes и обновлена сигнатура для сервиса, который эту ручку вызывает
- Исправлен нейминг некоторых тестов(get_groups, get_services)
- В тестах на группы, сервисы и note_type, способ удаления объектов заменен с dbsession.delete на кастомный метод(кроме мест, где проверяется атрибут is_deleted)
- Исправлен формат написания параметризации в некоторых тестах, для лучшей читаемости
- Исправлена критическая ошибка в тестах test_groups::test_post_group и test_services::test_post_service с всегда истинным ассертом
- Все тесты структурированы по модели CRUD
2. Исправления по комментам клода
- Пункты 3, 6, 7, 8, 9 (test_get_notes)
- Исправлена и упрощена логика проверки сортировки модалок
- Реализован подход с тестированием контракта сервиса отвечающиего за фильтрацию и пагинацию
- Удален параметр len_without_confines, упразлнен подход с хардкодом ожидаемого результата
- Пункт 5 (галлюцинация) у метода model_validate так же есть параметр extra(проверил, что работает) ссылка на доку:
https://pydantic.dev/docs/validation/latest/api/pydantic/base_model/#pydantic.BaseModel.model_validate
- Пункт 11 Исправлена мутация тестового body
- Пункт 12
- Добавлена общая временная точка в conftest.py
- Исправлены временные метки для создаваемых модалок (Сортировка для равных временных меток не
детерминирована в самой бизнес-логике, поэтому проверить сортировку в таком случае не будет возможно.
Сейчас для каждой модалки создана уникальная временная метка)
- Добавлена фикстура mock_datetime_now для ручки update_status(в сервисе вызывается datetime.now), чтобы тестах зависимых от вермени оно было единой точкой отсчета.
574fd17 to
09fd12e
Compare
|
|
||
| if status_code == status.HTTP_200_OK: | ||
| response_data = response.json() | ||
| response_model = GroupGet(**response_data) |
There was a problem hiding this comment.
а почему ты здесь не делаешь mdoel validate с forbid? тут все ведь аналогично services
ну это уже так, просто коммент на будущее
There was a problem hiding this comment.
Сорри, просто упустил этот момент
2 participants
Изменения
group.py,service.pyиnotes.pynote.py::get_notes, чтобы ограничить возможность передачи отрицательныхlimitиoffsetи точно валидировать строкуstatus.CRUD.test_get_notes). В тесте реализован подход эталонной логики(повторяем бизнес-логику в тесте и на основе неё проверяем), вместо хардкода ожидаемого результата.note_typeДополнительно
model_validateс параметромextra="forbid"в некоторых тестах(было предположение, что не работает - проверку наличия полей выполняет)https://pydantic.dev/docs/validation/latest/api/pydantic/base_model/#pydantic.BaseModel.model_validate
Check-List
blackиisortдля Back-End илиPrettierдля Front-End?